Skip to content

fix(workspace): name a workspace by its resolved path - #1798

Draft
ningmao-hlyz wants to merge 32 commits into
Agent-Field:devfrom
ningmao-hlyz:fix/1761-symlink-workspace-keys
Draft

ningmao-hlyz wants to merge 32 commits into
Agent-Field:devfrom
ningmao-hlyz:fix/1761-symlink-workspace-keys

Conversation

@ningmao-hlyz

Copy link
Copy Markdown

Fixes #1761.

What changed

A folder reached through a symlink became two projects, two conversation
histories and two engines. Both doors where a person's path becomes a key now
resolve symlinks with filepath.EvalSymlinks before keying:

  • engineWorkspace (cmd/codeaf/engine.go) — the resolver every engine
    command goes through (--daemon, --status, --stop) and the source of
    the host's workspace name. It stopped at filepath.Clean, so
    sha256(Clean(...)) in enginehost.where minted one host directory per
    spelling: --status <link> missed the engine started at <dir>.
  • /project (internal/tui3/projectcmd.go) — the pin stored the raw typed
    spelling, so home keyed the folder under the alias while a launch from the
    resolved path (already canonicalized by v3OpenTarget) keyed it under the
    target: two project rows, one folder.

Regression test: cmd/codeaf/engine_symlink_test.go drives a symlink through
engineWorkspace and asserts both spellings land on the same directory, and
that a plain path still resolves to itself.

Changelog entry: docs/rules/changelog.md names the entry for its pull
request, and the number exists only once the pull request does — so it
arrives in the next push as
docs/changes/unreleased/<this-pr>-symlink-workspace-keys.md
(kind: fixed, surface: [engine, workspace]). The first red on check
is that workflow, not a mistake.

How it was checked

  • go build ./cmd/codeaf ./internal/tui3 — exit 0 (go1.26.5).
  • go test ./cmd/codeaf -run TestEngineWorkspace -count=1 -v — both tests
    pass: TestEngineWorkspaceResolvesSymlink,
    TestEngineWorkspaceKeepsNonSymlinkPathUnchanged (ok … 2.434s). To run
    this on Windows the other cmd/codeaf test files were moved aside for the
    run and put back — see the limitation below.
  • GOOS=linux go vet ./cmd/codeaf ./internal/tui3 — exit 0, the platform CI
    runs, so every test file (including the added one) typechecks there.
  • gofmt on the committed blobs — clean (engine.go, projectcmd.go,
    engine_symlink_test.go).
  • Reproduction at base 8761488a: a stdlib probe mirroring enginehost.where
    showed sha256(Clean(<link>)) ≠ sha256(Clean(<real>)) — two spellings,
    two host directories — and that EvalSymlinks collapses both spellings to
    one host directory.

Honest limitation: cmd/codeaf's test package does not build on Windows at
all — carried_child_test.go uses fakeFolderProgram, which lives behind
//go:build !windows; the identical failure exists at base 8761488a. So
the full suite cannot run on my Windows checkout and defers to CI on Linux
(where GOOS=linux go vet passes).

Checklist

  • A change entry — arrives in the next push, named for this pull request
    (the number does not exist until the pull request does; see
    docs/rules/changelog.md).
  • The manual knows about it — no slash command, key, tool, default, limit
    or refusal moved (keying fix only), so internal/manual/chat/ is
    unchanged.
  • No new line in .github/known-red.txt.
  • Only my own paths are staged — cmd/codeaf/engine.go,
    cmd/codeaf/engine_symlink_test.go, internal/tui3/projectcmd.go.

@CLAassistant

CLAassistant commented Oct 9, 2026 •

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

Copy link
Copy Markdown
Member

Hey, thanks for the PR. Could you sign the CLA when you get a chance so we can review it?

ningmao-hlyz added a commit to ningmao-hlyz/CodeAF that referenced this pull request Oct 9, 2026
A folder reached through a symlink became two projects, two conversation
histories and two engines: engineWorkspace and /project stopped at
filepath.Clean, so each spelling hashed to its own enginehost host
directory and its own project row.

Resolve symlinks (filepath.EvalSymlinks) at both doors where a person's
path becomes a key: engineWorkspace (--daemon/--status/--stop and the
host's workspace name) and runProjectCommand (the project pin).

Fixes Agent-Field#1761
@ningmao-hlyz
ningmao-hlyz force-pushed the fix/1761-symlink-workspace-keys branch from fd94fdf to fa7d438 Compare October 9, 2026 16:50
@ningmao-hlyz

Copy link
Copy Markdown
Author

Hey, thanks for the PR. Could you sign the CLA when you get a chance so we can review it?

done

@ZeroPoint95 ZeroPoint95 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @ningmao-hlyz, thanks for working on this fix!

I’m requesting changes because resolving the path in only some entry points introduces inconsistent workspace identities.

  1. Resolve the local chat workspace before dialing or spawning. In a non-Git folder reached through a symlink, PWD can preserve the alias. The local surface still hashes that alias, while the daemon now resolves it and listens under a different key. The connection fails and chat falls back to running in the terminal. Please use the same resolved workspace for host lookup, dialing and spawning.

  2. Resolve the default home directory too. engineWorkspace("") returns HOME before reaching the new canonicalization. When HOME is a symlink, omitting --workspace and explicitly passing the same home now select different hosts. Please route the default through the same resolution.

  3. Update the affected tests and cover these entry points. Five existing macOS tests now fail because their fixtures or expectations use unresolved paths, including two failures in make test-laws. Please normalize the fixture paths and expected results, and add symlink regressions for local host startup, default HOME, and typed /project.

Both functional reproductions fail at this head and pass on the base. Builds, vet, changelog validation and packed-manual checks passed. CI is green, but the local laws and race runs expose the failures above; unrelated failures were separately confirmed on the base.

@ningmao-hlyz

Copy link
Copy Markdown
Author

Thanks for the thorough review, and for taking the time to run the repros at this head.

All three points are confirmed as real gaps in my patch:

  1. Local chat hashing the unresolved PWD alias while the daemon listens on the resolved key - agreed, host lookup, dialing and spawning need to share one resolved workspace.
  2. The default HOME path skipping the canonicalization entirely - agreed, engineWorkspace("") needs to go through the same resolution.
  3. The five macOS test failures and missing coverage - I will normalize the fixtures and add symlink regressions for local host startup, default HOME and typed /project.

Working on the fixes now; I will push as incremental commits and follow up here with the new SHA once all three points are addressed.

Every identity question in this package — host lookup, dial, spawn, lock,
status, stop — funnels through where, so where is the one place a symlinked
spelling must resolve. A surface that reached the folder through a link and a
daemon named by the resolved path hashed to two host directories, and the
launch spent its wait failing to meet (Agent-Field#1761). A path that does not resolve —
a workspace not on disk yet, a test's imagined directory — keeps its spelling.
engineWorkspace("") returned home without the resolution a typed home goes
through, so a home behind a symlink keyed two hosts depending on whether
--workspace was omitted or spelled out (Agent-Field#1761). The default now walks the same
door.

And the launch-side regression: a surface that dials through a symlinked
spelling must meet the host standing at the folder it reaches.
A pin names the folder the link reaches, not the spelling that was typed —
the typed-path half of Agent-Field#1761's regression, beside the daemon and host halves
already on this branch.
@ningmao-hlyz

Copy link
Copy Markdown
Author

All three points are addressed at d26cae9, pushed as three incremental commits on top of the dev merge.

  1. Local chat hashing the alias. The split is gone at the identity layer rather than at any entry point: every host question in internal/enginehost - lookup, dial, spawn, lock, status, stop - funnels through one place, and that place now resolves the spelling before it hashes. A surface dialing through a symlink and a daemon named by the resolved path can no longer build two host directories. Regression: a host standing at the folder answers a dial that arrives through the link (cmd/codeaf), and the two spellings get one socket and one lock (internal/enginehost).

  2. The default home. engineWorkspace("") walks the same resolution a typed home does now, so omitting --workspace and naming the same home cannot key two hosts. Regression covers exactly that pair.

  3. The five macOS failures. I believe they share the same root as point 1 rather than being fixture bugs: the tests hand one temp-directory spelling to the child they spawn and compute the socket from the same unresolved string themselves, so the child daemon (which resolves through engineWorkspace) listened one key away from where the test attached. The identity unification removes that split for every such test, and I could not run macOS locally to verify - could you re-run make test-laws and the race run at this SHA? If any failure survives, it is a genuine fixture mismatch and I will fix those files separately.

Also added the typed /project regression the review asked for (internal/tui3). Builds, gofmt, GOOS=linux vet and the touched suites pass here; the full cmd/codeaf suite still defers to Linux CI for the same Windows limitations stated in the PR description.

The surface's own answer — v3Workspace — minted the session bucket and the
tools root from whatever spelling the terminal walked in with, so on the
in-process doors one folder reached through a symlink was still a second
project beside the same folder named by its resolved path. The review of
Agent-Field#1798 caught the host half of that split; this closes the surface half the
same way, at the one place a project key is minted.
ProjectKey's own doc calls the fallback "the canonical path's hash", but
filepath.Abs canonicalizes nothing about links — a folder reached through a
symlink keyed two memory projects (Agent-Field#1761). EvalSymlinks after Abs, with the
absolute spelling kept when the path does not resolve.
The entry described the engineWorkspace door alone. The review pass that
followed spread the resolution across the host identity, the default home,
the surface's project key and the memory path fallback — the entry says so,
because a memory that says "fixed at engineWorkspace" would send the next
reader looking for one door and finding four.
@ningmao-hlyz

Copy link
Copy Markdown
Author

A second pass over the identity question before you re-run the suites, and it caught two doors the first round left split - both now resolved at ac09e71.

  1. The surface's own project key. v3Workspace minted the session bucket and the tools root from whatever spelling the terminal walked in with, so on the in-process doors (--once, --no-host, the fallback road) one folder reached through a symlink was still a second project beside the same folder named by its resolved path - the exact "two histories" complaint of workspace: a folder reached through a symlink becomes two projects and two engines #1761, one layer up. The answer now resolves at that one mint, which also keeps the projects panel and the drafts keys on one spelling, since everything downstream reads the workspace from here.

  2. Memory's path fallback. gitidentity.ProjectKey's doc calls its fallback "the canonical path's hash", but filepath.Abs canonicalizes nothing about links - a non-git folder behind a symlink keyed two memory projects. The fallback now resolves, matching what the doc already promised.

Both carry regressions that fail on the previous head and pass here, and the changelog entry now names every door that mints a key instead of just the first one. tui3's full suite, the touched cmd/codeaf suites, gofmt and the changelog check are green here; the Windows-only failures this machine shows are the same set the base shows.

That closes every place a path becomes a key that I could find: engineWorkspace (named and default), enginehost.where, v3Workspace, gitidentity, and the /project pin. If your macOS run still surfaces a fixture mismatch, it will be a genuine one now.

@santoshkumarradha santoshkumarradha added bug Something the code does that it should not area:session The engine — turns, tasks, the toolbelt, checkpoints sev:serious Wrong or missing behaviour a person meets in ordinary use labels Oct 14, 2026
@santoshkumarradha santoshkumarradha added this to the Reliable agent milestone Oct 14, 2026
@ZeroPoint95

Copy link
Copy Markdown
Collaborator

Hi @ningmao-hlyz, thanks for the follow-up and the additional fixes!

I rechecked 16643c4ad. The local-dial and default-HOME fixes work, and the environment-status test now passes. Three points still need changes:

  1. Preserve access to engines started by older builds.
    The new canonical host hash does not accommodate existing alias-keyed directories. In an upgrade fixture, Held() lists the old engine, but Dial() misses its live socket and Stop() returns false, nil. This also prevents --status-all, --stop-all and stale-host takeover from reaching it. Please preserve lookup and retirement of legacy hosts whose recorded workspace resolves to the same folder.

  2. Preserve resumption of existing conversations.
    A spoken conversation stored in the old alias-keyed bucket no longer resumes: v3Workspace → v3ResolveSession selects the canonical bucket and creates a new journal. Please preserve the ability to resume these conversations through compatibility lookup or a targeted migration, and add an upgrade regression test. A comprehensive merge of historical buckets is not required here.

  3. Fix the four remaining macOS fixture failures.
    Both hello tests in hostlatest_test.go, the empty-workspace status test in engine_stale_test.go, and the typed /project test in homefate_test.go still fail. The hello fixtures populate unresolved buckets; the status and pin expectations retain unresolved paths. Please normalize these fixtures and expectations while preserving their substantive assertions.

The upgrade reproductions and all four fixture failures fail at this head and pass on the exact base. Builds, vet, packed-manual and changelog checks passed. CI is green, but local laws and race runs still expose these fixture failures; unrelated failures were separately confirmed on dev.

…s host

The host directory is named by a hash of the workspace, and once that hash was
taken from the resolved spelling (Agent-Field#1761), a directory an older build named by
the raw spelling it was handed went dark for every door here: Held reads what
was written inside, but Dial, Stop, the status door and the takeover rule ask
by key - an upgrade stranded a running engine and the commands that must stop
it. A canonical miss is now answered by host directories whose recorded
workspace resolves to the same folder, and the canonical error is what the
caller still sees when nothing anywhere answers.
… resume

Buckets minted before the project key was resolved (Agent-Field#1761) carry the spelling
the terminal walked in with, and a launch that reads only the canonical bucket
opened a fresh journal beside every conversation a person already had. When
the canonical bucket holds nothing spoken, the newest folder in another bucket
whose meta.workspace citation resolves to this folder resumes where it lives -
no migration, no merge; the canonical key takes over from there.
The two hello fixtures populated a bucket by the raw temporary-directory
spelling while the doors they exercise resolve it; the status expectation and
the typed /project pin expectation kept the raw spelling too. On macOS, where
every temporary root is reached through a link, all four failed. The fixtures
and expectations now stand where the current build stands, and their
substantive assertions are unchanged.
The entry now says how: host directories an older build named by the raw
spelling stay reachable through their recorded workspace, and conversations in
buckets minted the same way resume through the workspace their folders cite.
@ningmao-hlyz

Copy link
Copy Markdown
Author

All three points are addressed at d214013, four incremental commits on top of your merge.

  1. Legacy hosts. A Dial that misses the canonical key now looks for host directories whose recorded workspace (the place file) resolves to the same folder, so Held's answer and the socket the doors dial are one fact again: --status, --stop, --status-all, --stop-all and the takeover rule all reach an engine the upgrade did not start, and the canonical dial error is what the caller still sees when nothing anywhere answers, so DialStartingHost's retry semantics do not move. Regression: a legacy directory keyed by the raw spelling with a live socket is met by a dial from the resolved folder.

  2. Legacy conversations. When the canonical bucket holds nothing spoken, the newest folder in another bucket whose meta.workspace citation resolves to this folder resumes where it lives - no migration, no merge; the canonical key takes over from there. The same lookup backs the engine's hello key, so a plain hello joins the held conversation instead of booting a second agent onto its journal. Upgrade regression covers both readings, resumption and bucket.

  3. The four fixtures. The two hello fixtures now populate the bucket the current build writes and stand on the resolved spelling; the empty-workspace status expectation and the typed /project pin expectation follow the same resolution. Their substantive assertions are unchanged - the hello-key-to-boot agreement, the none-sentence shape, the pin-lands-and-says-nothing behavior.

Verification here: both new regressions fail on your merge and pass at this head; the full enginehost, tui3 and cmd/codeaf failure sets are byte-identical to your merge's on this machine apart from the four fixtures this commit fixes and one known-flaky walker test; builds, gofmt, GOOS=linux vet and the changelog check are green. The changelog entry now also states the upgrade road, since a session reading "every key resolves" would otherwise not know the old records stay readable.

AbirAbbas and others added 3 commits October 20, 2026 08:45
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Keep upgrade retirement behind the original host lock until its journals finish closing. Exercise status, stop, sweeps and idle or busy takeover through both workspace spellings.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Use the same spoken-conversation priority for hello discovery and boot. Prove that two real socket connections reuse one agent for an alias-keyed conversation even when the canonical bucket contains an empty session.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@AbirAbbas

Copy link
Copy Markdown
Collaborator

@ningmao-hlyz thanks for working through two rounds on this. Since nobody had re-reviewed after your last push, I've taken it over. I verified each point and pushed two fixes on top of your branch (no history rewritten).

@ZeroPoint95, all three of your points from the 14th:

  1. Engines started by older builds. I rebuilt your upgrade fixture: an alias-keyed host dir with a live socket and a real flock. Held, Dial, Stop, --status, --status-all, --stop-all and idle/busy takeover all reach it from both spellings (10 door × spelling cases), and the built binary passes the same check. That turned up one more gap. Once the old socket closed, retirement only checked the new canonical lock, so it could report success while the old engine was still holding its lock and flushing journals. Retirement now waits for the legacy lock too (73262a5). The test fails on the previous head and passes now.
  2. Existing conversations. An old alias-bucket conversation resumes in place through v3Workspace → v3ResolveSession. One edge case remained: if an empty canonical conversation existed, hello picked it while boot resumed the spoken legacy one, and two real socket hellos booted two agents onto the same journal. Hello now uses the same spoken-history priority as boot (58b3a98), and the regression shows one boot with two connections. Canonical spoken history still wins, and explicit sessions are unchanged.
  3. macOS fixtures. Running with TMPDIR and HOME both pointed through symlinks (the /var → /private/var shape), both hostlatest hello tests, the empty-workspace status test and the typed /project test pass, with their assertions intact. The old versions of those fixtures reproduce your four failures in the same setup.

I also merged current dev. The PR gate passes on Go 1.26.5: build, vet, fmt, packed manual, changelog, laws, carried help and belt pages, plus the codeaf / rest / tui3 legs. The only noise was a TempDir cleanup flake in an unrelated shell subtest, which passes on retry and on dev. On the security side, the compatibility lookup only matches a recorded workspace whose EvalSymlinks equals the requested folder. It skips symlinked state dirs, and foreign, missing or looping citations never match. A Dial miss scans the host dirs linearly, which is fine for normal inventories; I'd only index it if someone accumulates hundreds.

@AbirAbbas
AbirAbbas requested a review from ZeroPoint95 October 20, 2026 13:54
AbirAbbas
AbirAbbas previously approved these changes Oct 20, 2026

@AbirAbbas AbirAbbas left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verified at 58b3a98: alias-keyed hosts and conversations from older builds stay reachable from both spellings, the two edge cases found during takeover (legacy-lock retirement, hello vs boot priority) are fixed with failing-before tests, and the four macOS-shape fixtures pass under symlinked TMPDIR/HOME. PR gate green.

ningmao-hlyz and others added 14 commits October 21, 2026 01:04
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Use LaunchDir only for owned conversations so hello and boot find their old HOME or temporary bucket and retain their private workspace.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…ogether

Use one directory inventory for Attach, Run, startup probing and retirement. Recheck under the canonical lock and retain legacy locks for the host lifetime so a listener gap cannot allow a second engine.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Accept either citation for owned conversations while keeping launch-only matches unavailable to borrowed sessions. Cover both HOME and temporary owned launches.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Carry canonical and historical owner keys through launch, resume, anchoring and workers. Read and edit old rows in place while keeping new writes canonical, including non-Git folders and repositories without origin.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Search canonical and cited historical prefixes together. Match local workspace and transcript aliases while preserving machine, conversation, live-window and newest-record guards; save restored composers before pruning their old records.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Use the same resolved path classification at launch and sweep, including symlinked HOME and TMPDIR outside /tmp. Retain the temporary citation and verify the sweep can still classify it.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Describe owned launch citations, path-memory aliases, draft restoration, lifetime-lock handoff and normalized temporary provenance without promising a merge of historical buckets.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Keep the store graph owner invariant for corrections, with new independently remembered lines still written canonically. Carry a proven raw workspace through the existing local hello field so flat-session memory owners stay readable without metadata.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Retain validated launch spellings in the surface even when a flat session has no metadata citation. Propagate that read provenance with each composer and clear it alongside draft storage on takeover.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Use a physical target in the citation-filtering fixtures so macOS temp-root symlinks do not introduce an extra valid incoming alias into their expected sets.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@AbirAbbas

Copy link
Copy Markdown
Collaborator

Status update: I'm holding this one and moving it back to draft. Neither the original head nor the current head is safe to merge.

The fix itself is right: a folder reached through a symlink should be one workspace. The hard part is the upgrade. Two independent reviews found that changing the key with nothing else hides data existing users already have:

  • path-owned project memories drop out of the project's scope,
  • owned conversations under a symlinked HOME stop resuming,
  • saved unsent drafts aren't found,
  • a starting legacy engine doesn't block a second engine on the same folder.

I pushed a round (79b8171..de11a6b) that reads old keys as aliases when a recorded citation resolves to the same folder today. A third review found that approach is unsafe too, because a symlink can be repointed. With current -> A retargeted to B, opening B by its plain path pulls in A's historical memories, including correct and forget on them. It also restores A's unsent draft into B and refuses to start B's engine while A's is running. Separately, the CLI normalizes the workspace before taking the lock, so the raw key never gets locked in the window before a legacy daemon starts listening.

This needs a decision before more code: old keys can only be trusted if the project that owned them is recorded durably. The options I see:

  1. Durable association. When a host, conversation, memory owner or draft is created, record the physical target (or a stable project ID) next to it. Treat an old key as an alias only when that recorded target matches.
  2. One-time migration. At first launch after upgrade, re-own path-keyed memories, drafts and buckets whose recorded workspace resolves to the current folder, and record that the migration ran. Never re-resolve later.
  3. Narrow scope. Canonicalize only the engine host key, which is what workspace: a folder reached through a symlink becomes two projects and two engines #1761's duplicate engines are about, and leave conversation, memory and draft keys alone for now.

@ZeroPoint95, which direction do you want? The reproductions for every finding above are written as Go tests, and I'll attach them to whichever route you choose.

@AbirAbbas
AbirAbbas marked this pull request as draft October 20, 2026 18:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:session The engine — turns, tasks, the toolbelt, checkpoints bug Something the code does that it should not sev:serious Wrong or missing behaviour a person meets in ordinary use

Projects

None yet

Development

Successfully merging this pull request may close these issues.

workspace: a folder reached through a symlink becomes two projects and two engines

5 participants